-
Notifications
You must be signed in to change notification settings - Fork 85
feat: Remove ANSI control chars from GitHub step Summary and PR comment #1312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
update branch
more tests formatting
fix formatting
|
Hi @tgummerer, do you by any chance have time to take a look at this PR? |
|
Hi all This one contains some real QoL improvements |
src/libs/summary.ts
Outdated
| import * as core from '@actions/core'; | ||
| import { Config } from '../config'; | ||
|
|
||
| function trimOutput( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why trimOutput here and in pr.ts? Looks like one of these should be removed they're doing the same thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, they do the same thing, yes, they trim the input string to the desired format, but for pr and summary, the formats are different.
Summary has a limit on the maximum message size in bytes, and pr has a limit on the maximum length in characters.
Maybe I should use the different names to avoid confusion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, ready for re-review
extracted ansi regexp
tgummerer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about dropping the ball on this, and taking so long to review it. This PR looks good to me now, I'm gonna get it merged.
|
This PR has been shipped in release v6.6.0. |
This PR introduces striping of ANSI symbols for GitHub step Summary and improves the current one for PR comments.
To resolve #1248
This is similar to #1231, but uses a slightly different approach. Instead of converting ANSI symbols to html tags (using the
ansi-to-htmlpackage), which results in\x1b[30mblackbecoming<span style="color:#000">black</span>, we simply remove them using a regular expression, which results in\x1b[30mblackbecoming justblack.The reason is that neither the Summary nor the PR comment is able to actually render the
<span style="color:#000">block with color. However, all these html tags consume limits for PR comment length and Summary size.Fixes #1387